Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AIP-84 Fix default ordering when directly using SortParam #44393

Conversation

pierrejeambrun
Copy link
Member

@pierrejeambrun pierrejeambrun commented Nov 26, 2024

This should fix the flakiness observed in main.

When no order is specified (i.e None), this should use the primary key as the default sorting. (Exactly what we do to fill the query parameter default value, but in some cases we use the SortParam outside of the FastAPI dependency system, bypassing the dynamic_depends function).

@pierrejeambrun pierrejeambrun added the AIP-84 Modern Rest API label Nov 26, 2024
@pierrejeambrun pierrejeambrun added this to the Airflow 3.0.0 milestone Nov 26, 2024
@pierrejeambrun pierrejeambrun self-assigned this Nov 26, 2024
jason810496 added a commit to jason810496/airflow that referenced this pull request Nov 26, 2024
jason810496 added a commit to jason810496/airflow that referenced this pull request Nov 26, 2024
Copy link
Contributor

@amoghrajesh amoghrajesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch

@pierrejeambrun
Copy link
Member Author

Merged, @uranusjr feel free to rebase and report back if the issue persists.

@pierrejeambrun pierrejeambrun merged commit e1ff64f into apache:main Nov 26, 2024
46 checks passed
@pierrejeambrun pierrejeambrun deleted the aip-84-fix-default-ordering-when-directly-using-sort-param branch November 26, 2024 16:48
@potiuk
Copy link
Member

potiuk commented Nov 26, 2024

nice!

pierrejeambrun pushed a commit that referenced this pull request Nov 27, 2024
* Refactor get_connections

* Allow Column type for `to_replace` parameter

* Refactor get_dags

* Refactor get_import_errors

* Refactor SortParam, get_dag_runs

* Fix default ordering when directly using SortParam

- related: #44393

* Fix get_list_dag_runs_batch
ArshiaZr pushed a commit to ArshiaZr/airflow that referenced this pull request Nov 27, 2024
* Refactor get_connections

* Allow Column type for `to_replace` parameter

* Refactor get_dags

* Refactor get_import_errors

* Refactor SortParam, get_dag_runs

* Fix default ordering when directly using SortParam

- related: apache#44393

* Fix get_list_dag_runs_batch
LefterisXefteris pushed a commit to LefterisXefteris/airflow that referenced this pull request Jan 5, 2025
LefterisXefteris pushed a commit to LefterisXefteris/airflow that referenced this pull request Jan 5, 2025
* Refactor get_connections

* Allow Column type for `to_replace` parameter

* Refactor get_dags

* Refactor get_import_errors

* Refactor SortParam, get_dag_runs

* Fix default ordering when directly using SortParam

- related: apache#44393

* Fix get_list_dag_runs_batch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AIP-84 Modern Rest API
Projects
Development

Successfully merging this pull request may close these issues.

5 participants